Review: feat(examples): add CodeBuddy Code CLI integration example (PR #996)

This review is AI-generated and does not represent human approval.

Overview

This PR adds a substantial CodeBuddy Code CLI integration example (5,629 additions across 25 files). It includes a Docker image, host-side executor scripts, an MCP server, a CodeBuddy bash-routing plugin with installer, comprehensive documentation in both English and Chinese, a CI workflow, and a 166-test pytest suite. The architecture follows the sound "keep the LLM agent on the host, route dangerous operations into a disposable VM" pattern.

The overall quality is high — the code is well-structured, security-conscious (path validation, symlink rejection, session file hardening, credential separation), thoroughly tested, and well-documented. Below are issues I recommend addressing.

Findings (ranked by severity)

Medium

1. CI workflow leaks container on exec failure (codebuddy-integration.yml:75-86)

The workflow starts a daemon container with --rm, then runs docker exec commands, then docker rm -f "$cid". If any docker exec fails (e.g., codebuddy --version returns non-zero), the shell exits before reaching the cleanup line, leaving the container running. The --rm flag only removes the container after its main process exits, which won't happen for a daemon.

Failure scenario: A transient npm install failure causes codebuddy --version to fail. The CI step aborts without cleanup, and the container keeps running on the runner until the job times out, consuming resources.

Suggested fix: Add a trap at the top of the run: block:

cid=""
cleanup() { [ -n "$cid" ] && docker rm -f "$cid" 2>/dev/null || true; }
trap cleanup EXIT
cid=$(docker run -d --rm codebuddy-cube:ci)

2. mcp_server.py duplicates _codebuddy_common.run_command (mcp_server.py:2563-2584)

The MCP server defines its own run_command() that returns a dict, while _codebuddy_common.run_command() returns an SDK result object. Both execute sandbox.commands.run() with similar error handling (catching CommandExitException). This means any bug fixes or improvements to the shared helper won't apply to the MCP server.

Suggested fix: Refactor the common run_command() to also offer a dict-return variant, or have mcp_server.py call _codebuddy_common.run_command() and convert the result.

3. Package name regex rejects valid PEP 508 names (sandbox_exec.py:3686)

The regex _PACKAGE_NAME_RE is:

r"^[A-Za-z0-9]([A-Za-z0-9._-]*[A-Za-z0-9])?$|^[A-Za-z0-9]$"

PEP 508 allows trailing ._- characters (e.g., foo_), but this regex requires the name to end with an alphanumeric character. The comment says "be conservative," which is a reasonable security stance, but this should be documented more explicitly as an intentional restriction. Users hitting this with valid package names may not understand why they're blocked.

Low

4. Container lifecycle: --rm is redundant with explicit docker rm -f (codebuddy-integration.yml:75,86)

The workflow passes --rm to docker run then explicitly calls docker rm -f "$cid" at the end. --rm only activates on container exit, not on explicit docker rm -f, so the flag has no effect in the happy path. Nonetheless, keeping it is harmless as a belt-and-suspenders for the failure path (even though, as noted in finding 1, the failure path doesn't reach either cleanup).

5. mcp_server.py lacks notifications/cancelled handler (mcp_server.py:2816)

The MCP protocol defines notifications/cancelled which is silently passed to the catch-all error handler (-32601). While the cancellation notification is optional for simple servers, the error response for a notification (which expects no response) is technically a protocol violation. This is a trivial fix — either add a handler that returns None or add it to the notifications/initialized guard.

6. _codebuddy_common.py imports SDK internals despite claiming SDK-agnosticism (_codebuddy_common.py:1540)

The module docstring says it's "SDK-agnostic (duck-typed on sandbox.commands.run)," but line 1540 imports CommandExitException from e2b.sandbox.commands.command_handle. This creates a hard runtime dependency on the e2b SDK internals. The catch in run_command() is pure convenience (swallowing non-zero exits), so this could be replaced with a broad except Exception to preserve SDK agnosticism.

Notes

7. Session file TOCTOU is acknowledged but not eliminated (sandbox_exec.py:3693-3701)

The code includes a detailed comment about the TOCTOU race between FileExistsError unlink() and the retry open(). The three-attempt retry with backoff mitigates but does not eliminate the window. The code also correctly notes that on shared clusters, per-invocation sandboxes (without --keep-alive) bypass this entirely. This is a well-documented and acceptable trade-off.

8. Input validation is thorough and correct

Path traversal prevention (_validate_path, exec_file), symlink rejection (O_NOFOLLOW + S_ISREG), command length caps (64 KB), timeout bounds (5 min), and credential stripping from proxy URLs are all implemented correctly. The execFile usage in the JS plugin avoids shell injection via argument passing.

9. Test coverage is strong

The 166-test suite covers both success paths and failure modes (empty input, oversized input, non-string input, invalid paths, symlink rejection, SDK errors, stream writing edge cases). Tests are fully offline via mocking.

Summary

This is a well-crafted integration example that demonstrates deep understanding of the CubeSandbox security model. The three medium findings (CI cleanup, code duplication, PEP 508 regex) are worth addressing. No blocking issues were found.